protoc-gen-go-grpc: add requirement of embedding UnimplementedServer in services #3657
Conversation
dfawley
added
Type: Feature
There was a problem hiding this comment.
Reviewed 14 of 30 files at r1, 17 of 17 files at r2.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @dfawley)
cmd/protoc-gen-go-grpc/grpc.go, line 142 at r2 (raw file):
} if *requireUnimplemented { g.P("unimplemented", serverType, "()")
Is there a better name so that the compilation error also serves as documentation?
requireUnimplementedXXX()?
needsToEmbedUnimplementedXXX()?
|
Good idea. I went with |
There was a problem hiding this comment.
Reviewed 17 of 17 files at r3.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @dfawley)
… would be an API change
| type LoadBalancerServer interface { | ||
| // Bidirectional rpc to get a list of servers. | ||
| BalanceLoad(LoadBalancer_BalanceLoadServer) error | ||
| mustEmbedUnimplementedLoadBalancerServer() |
There was a problem hiding this comment.
Oh, we said we want to revert the changes to the generated code, because they are API changes.
| files for gRPC. For usage information, please see our [quick start | ||
| guide](https://grpc.io/docs/languages/go/quickstart/). | ||
|
|
||
| By default, to register services using the methods generated by this tool, the |
There was a problem hiding this comment.
Move this to a separate section?
|
|
||
| This tool generates Go language bindings of `service`s in protobuf definition | ||
| files for gRPC. For usage information, please see our [quick start | ||
| guide](https://grpc.io/docs/languages/go/quickstart/). |
There was a problem hiding this comment.
Quick start doesn't have examples for using this plugin (yet?)
Even if it does, it would be helpful to have a copy-paste ready command here?
Or point to https://github.com/grpc/grpc-go/blob/master/regenerate.sh#L60
There was a problem hiding this comment.
Documentation is very much a work in progress. Let's make incremental progress on it until we eventually release the tool at 1.0.
dfawley
changed the title
No description provided.